Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[flake8-pyi] Rename PYI019 and improve its diagnostic message #15885

Merged
merged 1 commit into from
Feb 3, 2025

Conversation

AlexWaygood
Copy link
Member

@AlexWaygood AlexWaygood commented Feb 2, 2025

Summary

Currently the name of PYI019 is custom-type-var-return-type and its diagnostic message is:

"Methods like {method_name} should return Self instead of a custom TypeVar"

There are two issues that this PR addresses:

  • Often if you're converting a method to use Self rather than a custom TypeVar, you'll need to change other annotations in the method's non-self parameters, not just the self annotation and the return annotation. Indeed, our preview-mode autofix for stub files will change some of these annotations. But the message and name imply that the rule is just about the return annotation.
  • It should be pretty obvious which method the diagnostic is complaining about from the diagnostic's range, so I don't think we really need to mention the method name in the diagnostic message. It's more useful to mention the name of the TypeVar that we're suggesting the user should get rid of.

I think the changes here should all be backwards-compatible, but we'll need to remember to add a redirect for the documentation after the next release (whether it is a minor release or patch release). In my opinion, the diagnostic's range should also cover the whole of the function's header, but this would be backwards-incompatible, so I'll propose it separately (it will either need to be preview-only or wait until the next minor release).

Test Plan

cargo test -p ruff_linter --lib

@AlexWaygood AlexWaygood added the rule Implementing or modifying a lint rule label Feb 2, 2025
Copy link
Contributor

github-actions bot commented Feb 2, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@AlexWaygood
Copy link
Member Author

Strange, I would have expected some ecosystem hits here...

@MichaReiser
Copy link
Member

I assume you meant that we have to set up the redirect with the next release, whether it is a patch or minor release.

@AlexWaygood
Copy link
Member Author

AlexWaygood commented Feb 3, 2025

I assume you meant that we have to set up the redirect with the next release, whether it is a patch or minor release.

Correct, thank you! I'll update the PR description (edit: now updated)

@AlexWaygood AlexWaygood merged commit 9c64d65 into main Feb 3, 2025
21 checks passed
@AlexWaygood AlexWaygood deleted the alex/pyi019-message branch February 3, 2025 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants